Cut remote-DB list latency by batching N+1 lookups (banlist ~5×, comms ~6×) - #1533
Open
maxijabase wants to merge 14 commits into
Open
Cut remote-DB list latency by batching N+1 lookups (banlist ~5×, comms ~6×)#1533maxijabase wants to merge 14 commits into
maxijabase wants to merge 14 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the panel and MariaDB are geographically far apart, per-row SQL on list pages turns every RTT into wall-clock pain. This PR collapses those N+1 patterns into page-scoped batch queries, adds query-count instrumentation + regression tests, and tightens two batching edge cases (CONSOLE
RemovedBy = 0name resolution, and excludingaid = 0fromGetAllAdmins).Also included from the same deploy/debug cycle (not N+1, but already on this branch):
SB_SECRET_KEYvalidation (≥32 decoded bytes, shared PHP + entrypoint gate)config.php(not a possibly-stale env var).dockerignore+ Apache deny so localconfig.php/ backups /.envcannot bake into or be served from the imageMotivation
A production-shaped setup (app on Railway, MariaDB in Argentina) made public ban/comms lists take tens of seconds per full HTML response. The dominant cost was not “slow SQL” in isolation. It was many sequential round trips (admin names, mods, comments, banlog, demo flags, protest/submission side lookups, etc.) each paying transcontinental RTT.
What changed (DB performance)
Instrumentation
Sbpp\Db\Database::resetQueryCount()/getQueryCount()— increments on everyquery()(prepare), the single choke point for logical statementsBatched surfaces
UserManager::GetAllAdmins()aid <= 0(CONSOLE) so the cache matchesGetUserArraypage.banlist.phppage.commslist.phpadmin.admins.phpadmin.bans.phpadmin.groups.phpBatching edge cases + tests
RemovedBywith!== null(not!empty), so PruneBans / CONSOLERemovedBy = 0still enter the name map (BanlistRemovedByConsoleTest)GetAllAdminsexcludesaid <= 0(covered inUserManagerGetAllAdminsQueryCountTest)GetAllAdmins, and admin admins / protests / submissions / groupsMeasured query budgets stay flat across row counts (e.g. admin admins ≈5, protests ≈4, submissions ≈5, groups ≈4; caps set with small headroom).
Intentional follow-ups (out of this PR)
srvgroups.namerows can still amplify workGetAllAdminsDatabase::query()wrapper (e.g. raw PDO)Latency compare (before vs after)
Local harness: interleaved HTTP GETs against old production panel (
bans.electricservers.com.arwhich is runningghcr.io/sbpp/sourcebans-pp:2.0.2) vs new Railway deploy of this branch, same Argentina DB, full response body drained (not TTFB-only), auth cookies on both sides for admin routes.Notes for reviewers:
Also on this branch (not N+1)
.dockerignore- excludeweb/config.php,web/config.php.*(keepconfig.php.template), host.envfiles, vendor, caches, demos fromdocker/Dockerfile.prodcontext. Without theconfig.phpexclusion, a local./sbpp.shconfig (DB_HOST=db) can ship inside the image; the entrypoint skipsrender_configand containers fail withgetaddrinfo for db failedeven with correct env vars.Apache - deny
config.phpand backup suffixes (config.php.bak, …) via<FilesMatch>insbpp-prod.confso credential backups are not served as plaintext.JWT+prod-entrypoint.sh+ Docker quickstart -SB_SECRET_KEYmust be base64 that decodes to at least 32 bytes (JWT::signingKeyFromSecret,MIN_SECRET_BYTES = 32). Invalid base64 and short-but-valid base64 both fail closed with an operator-readable message (openssl rand -base64 47). The entrypoint validates the effective key inconfig.phpafter render (stale env cannot block a healthy install; fixed env cannot mask a bad persisted key). Fresh config writes still pre-check the env value before persisting it.Tests -
JwtSecretKeyTest,DockerIgnoreSecretsTest, expandedProdApacheConfigTest; compose mounts.dockerignorefor local PHPUnit symmetry with CI.Test plan
./sbpp.sh test --filter=QueryCount./sbpp.sh test --filter=UserManagerGetAllAdmins./sbpp.sh test --filter=BanlistRemovedByConsole./sbpp.sh test --filter=JwtSecretKey(invalid + short base64 rejected; 32-byte floor accepted)./sbpp.sh test --filter='ProdApacheConfig|DockerIgnoreSecrets'web/config.phpand confirm the baked image does not contain that fileconfig.php, and validates the key inside an existingconfig.phpeven when env differs